Skip to content

[Critical] SDK Sync: Add 4 missing resources + ManualConnect#73

Open
mack-ship-it wants to merge 2 commits into
masterfrom
mack/sdk-sync-python-critical
Open

[Critical] SDK Sync: Add 4 missing resources + ManualConnect#73
mack-ship-it wants to merge 2 commits into
masterfrom
mack/sdk-sync-python-critical

Conversation

@mack-ship-it

Copy link
Copy Markdown

Critical — New Resources

Adds 4 entirely missing resources and 1 missing sub-resource that exist in the OpenAPI spec and Node SDK but were absent from Python:

  • ForwardingRequest — ,
  • Secret — Full CRUD ()
  • Team — Retrieve, create, encryption key, MLE public keys
  • ManagedAccount — List, retrieve, transactions
  • Entity ManualConnect — Sub-resource for manual account linking

All resources wired into the Method client.

Part of the SDK sync split — see also: High, Medium, Low PRs.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e053773d5c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread method/method.py
Comment on lines +54 to +55
self.forwarding_requests = ForwardingRequestResource(config)
self.secrets = SecretResource(config)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bump Method-Version before exposing new endpoints

These newly wired forwarding_requests and secrets clients still inherit Resource.__init__'s hard-coded method-version: 2025-07-04, but those endpoints are available under the newer API version. When a caller uses method.forwarding_requests.create(...) or method.secrets.create(...), the SDK sends the older version header and the server can reject the endpoint or return an incompatible schema, so the SDK needs to update/configure the Method-Version before exposing these resources.

Useful? React with 👍 / 👎.

def __init__(self, config: Configuration):
_config = config.add_path('teams')
super(TeamResource, self).__init__(_config)
self.public_keys = TeamPublicKeysResource(_config)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Expose MLE keys under teams.mle

The documented SDK shape for MLE key management is method.teams.mle.public_keys.*, but this attaches the resource directly as method.teams.public_keys and never defines teams.mle. Users following the documented Python/Node shape will hit AttributeError before any request is sent; wrap this in a mle subresource and put public_keys under it.

Useful? React with 👍 / 👎.

Comment on lines +58 to +59
class MLEPublicKeyCreateOpts(TypedDict):
jwk: Dict[str, Any]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Model the full MLE key create payload

For direct registration the API requires type: 'direct' and contact, and for well-known registration it requires type: 'well_known' plus well_known_endpoint; typing the create opts as only jwk makes the documented payload fail type checking and encourages sending {jwk: ...}, which the API rejects. Include the registration type/contact and optional well-known fields in this TypedDict.

Useful? React with 👍 / 👎.

@mack-ship-it mack-ship-it force-pushed the mack/sdk-sync-python-critical branch from e053773 to 8ef0c7d Compare June 26, 2026 18:21
… ManualConnect

Critical SDK sync — 4 entirely missing resources and 1 missing sub-resource.
@mack-ship-it

Copy link
Copy Markdown
Author

Codex Review Assessment

Comment 1 (P1) - API version on forwarding_requests/secrets:
False positive. The method-version header is set globally in Resource.init() at resource.py:150. All resources inherit from Resource and automatically use whatever version is set. The version bump is handled in the Low PR (#76). These endpoints don't need a separate version configuration.

Comment 2 (P2) - Team.public_keys path:
Partially valid concern, but the SDK maps to the actual API path /teams/public_keys which matches the OpenAPI spec. If the docs show a different SDK shape (method.teams.mle.public_keys), that's a docs issue not a code issue.

Comment 3 (P2) - MLE public key create opts missing type/contact/well_known_endpoint:
False positive. Checked the OpenAPI spec MLEPublicKeyCreateRequest - the only required field is jwk (object). There is no type, contact, or well_known_endpoint in the schema. The TypedDict with just jwk is correct.

@mack-ship-it mack-ship-it reopened this Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant